Fix/plugin readme local assets#8360
Conversation
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- For the
/api/plugin/assetendpoint, consider avoiding passing the JWT via atokenquery parameter (used inReadmeDialog.vue) and instead reuse the existing auth mechanism (e.g., cookie orAuthorizationheader), since tokens in URLs are more likely to be logged or leaked via referrers and browser history.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- For the `/api/plugin/asset` endpoint, consider avoiding passing the JWT via a `token` query parameter (used in `ReadmeDialog.vue`) and instead reuse the existing auth mechanism (e.g., cookie or `Authorization` header), since tokens in URLs are more likely to be logged or leaked via referrers and browser history.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/shared/ReadmeDialog.vue" line_range="259-260" />
<code_context>
+ name: props.pluginName,
+ path: decodedPath,
+ });
+ const token = localStorage.getItem("token");
+ if (token) params.set("token", token);
+ return `/api/plugin/asset?${params.toString()}`;
+}
</code_context>
<issue_to_address>
**🚨 issue (security):** Avoid putting the auth token into image URLs if possible
Putting the dashboard JWT in the `<img>` `src` makes it more likely to leak via browser caches, server logs, referrers, and copy‑pasted URLs. If possible, have `/api/plugin/asset` rely on existing cookie/session auth (headers instead of query params), or use a short‑lived asset-specific token rather than the main dashboard token in the URL.
</issue_to_address>
### Comment 2
<location path="astrbot/dashboard/server.py" line_range="267-268" />
<code_context>
return None
is_plugin_page_path = PluginPageAuth.is_protected_path(request.path)
token = self._extract_dashboard_jwt()
+ if not token and request.path == "/api/plugin/asset":
+ token = request.args.get("token", "").strip()
if not token and is_plugin_page_path:
token = PluginPageAuth.extract_asset_token()
</code_context>
<issue_to_address>
**🚨 issue (security):** Using JWT from query string for `/api/plugin/asset` raises security and logging concerns
Passing the dashboard JWT as a `token` query param exposes it to logs, browser history, and referrers, increasing the risk of leakage or accidental sharing. Since this endpoint should be protected like the rest of the dashboard, prefer the existing auth mechanism (cookies/headers) or a dedicated short-lived/scope-limited token instead of reusing the main JWT in the URL.
</issue_to_address>
### Comment 3
<location path="dashboard/src/components/shared/PluginReadmeImageSourceSetting.vue" line_range="8" />
<code_context>
+ class="readme-image-source-switch"
+ color="primary"
+ density="compact"
+ hide-details="true"
+ :label="tm('network.proxySelector.readmeImages.useGitHub')">
+ </v-switch>
</code_context>
<issue_to_address>
**nitpick:** Use a boolean binding for `hide-details` instead of a string literal
`hide-details` is a boolean prop (or can be the special value `"auto"`). Using `hide-details="true"` passes a string, not a boolean. Prefer `:hide-details="true"` or just `hide-details` to keep the typing and behavior correct and explicit.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to load plugin README images either locally or from GitHub, including a toggle setting in the dashboard and backend routing to safely serve local assets. The review feedback highlights opportunities to improve security and code quality: specifically, adding stricter validation to GitHub repository URL parsing to prevent potential directory traversal, setting a Content-Security-Policy header on served assets to mitigate XSS risks from SVGs, and refactoring the new Vue component to use the modern <script setup> syntax and Composition API.
| def _parse_github_repo_url(self, repo_url: str | None) -> tuple[str, str] | None: | ||
| if not repo_url: | ||
| return None | ||
|
|
||
| parsed = urlsplit(repo_url.strip()) | ||
| if parsed.scheme not in ("http", "https"): | ||
| return None | ||
| if parsed.netloc.lower() != "github.com": | ||
| return None | ||
|
|
||
| parts = [part for part in parsed.path.strip("/").split("/") if part] | ||
| if len(parts) < 2: | ||
| return None | ||
|
|
||
| owner = parts[0] | ||
| repo = parts[1].removesuffix(".git") | ||
| if not owner or not repo: | ||
| return None | ||
| if not GITHUB_REPO_PART_PATTERN.fullmatch(owner): | ||
| return None | ||
| if not GITHUB_REPO_PART_PATTERN.fullmatch(repo): | ||
| return None | ||
|
|
||
| return owner, repo |
There was a problem hiding this comment.
为了提高代码的健壮性与安全性,建议在此处进行以下改进:
- 增加对
repo_url的类型检查,确保其为str类型,避免在传入非字符串类型时调用.strip()导致AttributeError。 - 显式地将
.和..排除在合法的owner或repo之外,防止潜在的目录穿越或不合规的 URL 解析问题。
def _parse_github_repo_url(self, repo_url: str | None) -> tuple[str, str] | None:
if not isinstance(repo_url, str):
return None
parsed = urlsplit(repo_url.strip())
if parsed.scheme not in ("http", "https"):
return None
if parsed.netloc.lower() != "github.com":
return None
parts = [part for part in parsed.path.strip("/").split("/") if part]
if len(parts) < 2:
return None
owner = parts[0]
repo = parts[1].removesuffix(".git")
if not owner or not repo:
return None
if owner in (".", "..") or repo in (".", ".."):
return None
if not GITHUB_REPO_PART_PATTERN.fullmatch(owner):
return None
if not GITHUB_REPO_PART_PATTERN.fullmatch(repo):
return None
return owner, repo| async def get_plugin_asset(self): | ||
| plugin_name = request.args.get("name") | ||
| asset_path = request.args.get("path") | ||
|
|
||
| if not plugin_name or not asset_path: | ||
| return await self._plugin_page_error_response(404, "Plugin asset not found") | ||
|
|
||
| plugin = self._get_plugin_metadata_by_name(plugin_name) | ||
| if not plugin: | ||
| return await self._plugin_page_error_response(404, "Plugin not found") | ||
|
|
||
| try: | ||
| file_path = await self._resolve_plugin_asset_file(plugin, asset_path) | ||
| except (FileNotFoundError, ValueError, OSError): | ||
| logger.warning(f"插件资源访问失败: {plugin_name}/{asset_path}") | ||
| return await self._plugin_page_error_response(404, "Plugin asset not found") | ||
|
|
||
| mimetype, _ = mimetypes.guess_type(file_path.name) | ||
| if not mimetype or not mimetype.startswith(PLUGIN_ASSET_MIME_PREFIX): | ||
| return await self._plugin_page_error_response(404, "Plugin asset not found") | ||
|
|
||
| return await self._serve_plugin_page_static_asset(file_path) |
There was a problem hiding this comment.
为了防止潜在的跨站脚本攻击(XSS),特别是当服务中允许加载并直接在浏览器中打开 SVG 格式的图片时,建议为该接口的响应头部添加严格的 Content-Security-Policy。
通过设置 Content-Security-Policy: default-src 'none',可以确保即使恶意的 SVG 图片中包含 <script> 标签,在浏览器中被直接打开时也无法执行任何脚本。
async def get_plugin_asset(self):
plugin_name = request.args.get("name")
asset_path = request.args.get("path")
if not plugin_name or not asset_path:
return await self._plugin_page_error_response(404, "Plugin asset not found")
plugin = self._get_plugin_metadata_by_name(plugin_name)
if not plugin:
return await self._plugin_page_error_response(404, "Plugin not found")
try:
file_path = await self._resolve_plugin_asset_file(plugin, asset_path)
except (FileNotFoundError, ValueError, OSError):
logger.warning(f"插件资源访问失败: {plugin_name}/{asset_path}")
return await self._plugin_page_error_response(404, "Plugin asset not found")
mimetype, _ = mimetypes.guess_type(file_path.name)
if not mimetype or not mimetype.startswith(PLUGIN_ASSET_MIME_PREFIX):
return await self._plugin_page_error_response(404, "Plugin asset not found")
response = await self._serve_plugin_page_static_asset(file_path)
response.headers["Content-Security-Policy"] = "default-src 'none'"
return response| <script> | ||
| import { useModuleI18n } from '@/i18n/composables'; | ||
| import { | ||
| PLUGIN_README_IMAGE_SOURCE, | ||
| getPluginReadmeImageSource, | ||
| setPluginReadmeImageSource | ||
| } from '@/utils/githubProxy'; | ||
|
|
||
| export default { | ||
| setup() { | ||
| const { tm } = useModuleI18n('features/settings'); | ||
| return { tm }; | ||
| }, | ||
| data() { | ||
| return { | ||
| readmeImageSource: PLUGIN_README_IMAGE_SOURCE.LOCAL, | ||
| initializing: true, | ||
| } | ||
| }, | ||
| computed: { | ||
| readmeImageUseGitHub: { | ||
| get() { | ||
| return this.readmeImageSource === PLUGIN_README_IMAGE_SOURCE.GITHUB; | ||
| }, | ||
| set(value) { | ||
| this.readmeImageSource = value | ||
| ? PLUGIN_README_IMAGE_SOURCE.GITHUB | ||
| : PLUGIN_README_IMAGE_SOURCE.LOCAL; | ||
| } | ||
| } | ||
| }, | ||
| mounted() { | ||
| this.readmeImageSource = getPluginReadmeImageSource(); | ||
| this.initializing = false; | ||
| }, | ||
| watch: { | ||
| readmeImageSource: function (newVal) { | ||
| if (this.initializing) { | ||
| return; | ||
| } | ||
| setPluginReadmeImageSource(newVal); | ||
| } | ||
| } | ||
| } | ||
| </script> |
There was a problem hiding this comment.
为了与项目中的其他组件(如 ReadmeDialog.vue 和 Settings.vue)保持一致,建议将此组件重构为 Vue 3 推荐的 <script setup> 语法和 Composition API。
这不仅能使代码更加简洁、易读,还能利用 watch 的默认行为(在初始化时不触发),从而优雅地省去 initializing 状态和 mounted 生命周期的手动控制逻辑。
<script setup>
import { ref, watch, computed } from 'vue';
import { useModuleI18n } from '@/i18n/composables';
import {
PLUGIN_README_IMAGE_SOURCE,
getPluginReadmeImageSource,
setPluginReadmeImageSource
} from '@/utils/githubProxy';
const { tm } = useModuleI18n('features/settings');
const readmeImageSource = ref(getPluginReadmeImageSource());
const readmeImageUseGitHub = computed({
get() {
return readmeImageSource.value === PLUGIN_README_IMAGE_SOURCE.GITHUB;
},
set(value) {
readmeImageSource.value = value
? PLUGIN_README_IMAGE_SOURCE.GITHUB
: PLUGIN_README_IMAGE_SOURCE.LOCAL;
}
});
watch(readmeImageSource, (newVal) => {
setPluginReadmeImageSource(newVal);
});
</script>
a4c4a7d to
9bd38ca
Compare
|
@sourcery-ai review |
|
@gemini-code-assist review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In ReadmeDialog.vue,
onMountedcallswindow.addEventListenerwithout thetypeof window !== 'undefined'guard you used inonUnmounted, which can break in non-browser/test environments; consider adding the same check around the listener registration. - The plugin asset API currently relies solely on
mimetypes.guess_type(file_path.name)to enforce image-only responses, which is extension-based; if plugins might ship images without standard extensions, you may want to harden this by either checking theContent-Typevia lightweight sniffing or by restricting allowed extensions explicitly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In ReadmeDialog.vue, `onMounted` calls `window.addEventListener` without the `typeof window !== 'undefined'` guard you used in `onUnmounted`, which can break in non-browser/test environments; consider adding the same check around the listener registration.
- The plugin asset API currently relies solely on `mimetypes.guess_type(file_path.name)` to enforce image-only responses, which is extension-based; if plugins might ship images without standard extensions, you may want to harden this by either checking the `Content-Type` via lightweight sniffing or by restricting allowed extensions explicitly.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/shared/ReadmeDialog.vue" line_range="194-195" />
<code_context>
+ readmeImageSourceVersion.value += 1;
+}
+
+onMounted(() => {
+ window.addEventListener(
+ PLUGIN_README_IMAGE_SOURCE_CHANGED_EVENT,
+ handleReadmeImageSourceChanged,
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Add a defensive window check in onMounted to avoid issues in non-browser environments.
In `onMounted`, `window.addEventListener` is called without the `typeof window !== "undefined"` guard used in `onUnmounted`. In non‑browser contexts (SSR, some tests), this can throw if `window` is missing or partially mocked. Please add the same guard in `onMounted` before calling `window.addEventListener` to keep behavior consistent and avoid runtime errors.
Suggested implementation:
```
onMounted(() => {
if (typeof window !== "undefined") {
window.addEventListener(
```
To complete this change, ensure that:
1. The `if (typeof window !== "undefined") {` opened above is properly closed inside the same `onMounted` callback, after the existing `window.addEventListener(...)` call. For example, if you currently have:
```ts
onMounted(() => {
if (typeof window !== "undefined") {
window.addEventListener(
PLUGIN_README_IMAGE_SOURCE_CHANGED_EVENT,
handleReadmeImageSourceChanged,
);
}
});
```
This keeps the behavior consistent with the `onUnmounted` guard and avoids runtime errors in non‑browser environments.
2. No additional behavior changes are introduced; the only difference should be that the event listener is not registered when `window` is unavailable.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to load plugin README images either locally or from GitHub, including proxy support. It adds a new backend endpoint /plugin/asset to serve local plugin images securely, implements GitHub repository URL parsing to construct raw asset URLs, and adds a new frontend setting to toggle between local and GitHub image sources. The feedback suggests making the GitHub URL parser more robust by also supporting www.github.com domains.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if parsed.netloc.lower() != "github.com": | ||
| return None |
There was a problem hiding this comment.
To make the GitHub repository URL parsing more robust, we should also support www.github.com in addition to github.com. Users often copy-paste URLs directly from their browser address bar, which may include the www. prefix.
| if parsed.netloc.lower() != "github.com": | |
| return None | |
| netloc = parsed.netloc.lower() | |
| if netloc not in ("github.com", "www.github.com"): | |
| return None |
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to load and render images in plugin README files, supporting both local serving (via a new /api/plugin/asset endpoint) and remote loading from GitHub (with proxy support). It adds a setting in the dashboard to toggle between these image sources, updates the Markdown rendering logic to rewrite image URLs accordingly, and includes robust path traversal checks and MIME-type validation for local assets. Feedback highlights two areas of improvement in astrbot/dashboard/routes/plugin.py: explicitly registering common image MIME types (like .svg and .webp) to prevent cross-platform loading issues on Windows systems, and lowering the log level from warning to info/debug when a plugin asset fails to load to prevent log pollution from broken image links.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| PLUGIN_ASSET_MIME_PREFIX = "image/" | ||
| GITHUB_DEFAULT_BRANCH_REF = "HEAD" | ||
| GITHUB_REPO_PART_PATTERN = re.compile(r"^[A-Za-z0-9_.-]+$") |
There was a problem hiding this comment.
在 Windows 系统上,Python 的 mimetypes 模块依赖于系统注册表。如果注册表中没有注册 .svg 或 .webp 等格式,mimetypes.guess_type 可能会返回 None,导致这些格式的图片在 Windows 上无法加载(返回 404)。
建议在模块初始化时显式注册这些常见的图片 MIME 类型,以确保跨平台兼容性。
PLUGIN_ASSET_MIME_PREFIX = "image/"
GITHUB_DEFAULT_BRANCH_REF = "HEAD"
GITHUB_REPO_PART_PATTERN = re.compile(r"^[A-Za-z0-9_.-]+$")
# 显式注册常见图片类型,防止 Windows 系统注册表缺失导致 guess_type 失败
mimetypes.add_type("image/svg+xml", ".svg")
mimetypes.add_type("image/webp", ".webp")| try: | ||
| file_path = await self._resolve_plugin_asset_file(plugin, asset_path) | ||
| except (FileNotFoundError, ValueError, OSError): | ||
| logger.warning(f"插件资源访问失败: {plugin_name}/{asset_path}") | ||
| return await self._plugin_page_error_response(404, "Plugin asset not found") |
There was a problem hiding this comment.
如果插件的 README 中包含失效的图片链接(这在第三方插件中很常见),每次打开 README 都会触发 logger.warning,这可能会导致日志中充斥大量的警告信息(日志污染)。
建议将此处的日志级别降低为 logger.info 或 logger.debug,或者仅在调试模式下输出。
| try: | |
| file_path = await self._resolve_plugin_asset_file(plugin, asset_path) | |
| except (FileNotFoundError, ValueError, OSError): | |
| logger.warning(f"插件资源访问失败: {plugin_name}/{asset_path}") | |
| return await self._plugin_page_error_response(404, "Plugin asset not found") | |
| try: | |
| file_path = await self._resolve_plugin_asset_file(plugin, asset_path) | |
| except (FileNotFoundError, ValueError, OSError): | |
| logger.info(f"插件资源访问失败: {plugin_name}/{asset_path}") | |
| return await self._plugin_page_error_response(404, "Plugin asset not found") |
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to load plugin README images either from the local plugin directory or directly from GitHub (with optional proxy support). It adds a backend endpoint /plugin/asset to serve local images securely, implements GitHub URL parsing to resolve raw image URLs, introduces a frontend setting to toggle this behavior, and includes comprehensive tests for the new functionality. No review comments were provided, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
插件 README 中使用相对路径引用的图片,之前在 WebUI 中不一定能正常加载。对于把截图或说明文档资源放在插件仓库、插件目录内的插件,这会导致文档展示不完整。
此改动增加了一种更稳妥、也更照顾低上传带宽服务器的 README 图片加载方式:
Modifications / 改动点
github_raw_base,前端可基于 GitHub 默认分支构造 raw 图片链接,避免硬编码main或其他分支名。Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Serve plugin README images from either local plugin directories or GitHub raw URLs based on user settings, with secure backend routing and updated frontend rendering.
New Features:
Tests: